Skip to content

feat: Add 'PermissionManager' module#745

Merged
NicolasBourdin88 merged 6 commits intomainfrom
feat/permission-manager
Apr 27, 2026
Merged

feat: Add 'PermissionManager' module#745
NicolasBourdin88 merged 6 commits intomainfrom
feat/permission-manager

Conversation

@NicolasBourdin88
Copy link
Copy Markdown
Contributor

@NicolasBourdin88 NicolasBourdin88 commented Apr 15, 2026

Add a 'PermissionManager' that allow you to easily manage permission across your application.

Depends on #738

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 15, 2026

PR Reviewer Guide 🔍

(Review updated until commit d8bb9c7)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Coroutine Leak

In waitUntilGranted, when status.isGranted is already true, the returned lambda executes action() immediately but never completes the actionFired job. This leaves the LaunchedEffect suspended indefinitely at actionFired.join(), causing a coroutine leak every time this code path is taken (e.g., when a user triggers an action while the permission is already granted).

return fun() {
    if (status.isGranted) {
        action()
    } else {
        launchPermissionRequest()
        actionFired.complete()
    }
}
Missing Remember Keys

The remember call on line 38 lacks keys. If permissionState or shouldDisplayRationale references change (e.g., during recomposition with different inputs), the stale remembered instance will be used. Add remember(permissionState, shouldDisplayRationale) to ensure correctness.

remember { SupportedApiPermissionManagerState(permissionState, shouldDisplayRationale) }

Comment thread gradle/core.versions.toml Outdated
Comment thread PermissionManager/proguard-rules.pro Outdated
Copy link
Copy Markdown
Member

@sirambd sirambd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to support required permissions as well

@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit 830a2eb

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/permission-manager branch from 830a2eb to fdc6f09 Compare April 16, 2026 06:38
@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit fdc6f09

@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit 1678945

@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit 6d914f3

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/permission-manager branch 2 times, most recently from b91aed2 to d8bb9c7 Compare April 16, 2026 14:09
@LunarX LunarX requested a review from Copilot April 16, 2026 14:11
@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit d8bb9c7

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new PermissionManager Android library module intended to centralize permission handling via Jetpack Compose + Accompanist, and wires it into the multi-module build.

Changes:

  • Registers the new :PermissionManager Gradle module and publishes a new version-catalog alias for it.
  • Introduces a Compose-first PermissionManagerState API with supported/unsupported-API implementations.
  • Adds accompanist-permissions to the version catalog and uses it in the new module.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
settings.gradle.kts Includes the new :PermissionManager module in the build.
gradle/core.versions.toml Adds Accompanist permissions dependency + a new infomaniak-core-permissionmanager catalog entry.
PermissionManager/build.gradle.kts New Android library module configuration and dependencies (Compose + Accompanist).
PermissionManager/src/main/kotlin/com/infomaniak/core/permissionmanager/PermissionManagerState.kt Public rememberPermissionManagerState entry point + PermissionManagerState API.
PermissionManager/src/main/kotlin/com/infomaniak/core/permissionmanager/SupportedApiPermissionManagerState.kt Supported-permission implementation backed by Accompanist PermissionState.
PermissionManager/src/main/kotlin/com/infomaniak/core/permissionmanager/UnsupportedApiPermissionManagerState.kt No-op implementation for permissions not applicable on the current API level.
PermissionManager/src/main/kotlin/com/infomaniak/core/permissionmanager/PermissionType.kt Adds PermissionType enum mapping app concepts to Android permission strings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@NicolasBourdin88 NicolasBourdin88 requested a review from LunarX April 16, 2026 14:19
@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

1 similar comment
@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/permission-manager branch from 9afdc49 to 03957b7 Compare April 22, 2026 12:55
@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

1 similar comment
@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/permission-manager branch from 559f0d7 to aee766a Compare April 24, 2026 13:53
@NicolasBourdin88 NicolasBourdin88 changed the base branch from main to remove-baseline-from-compose-lint-plugin April 24, 2026 13:54
@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/permission-manager branch from aee766a to bacbb2c Compare April 24, 2026 13:58
@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

@LunarX LunarX force-pushed the remove-baseline-from-compose-lint-plugin branch from 8f2e395 to 9f4a09c Compare April 27, 2026 07:11
Base automatically changed from remove-baseline-from-compose-lint-plugin to main April 27, 2026 07:15
@github-actions
Copy link
Copy Markdown

This PR/issue depends on:

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/permission-manager branch from bacbb2c to 3078686 Compare April 27, 2026 12:12
@sonarqubecloud
Copy link
Copy Markdown

@NicolasBourdin88 NicolasBourdin88 merged commit 3f48259 into main Apr 27, 2026
7 checks passed
@NicolasBourdin88 NicolasBourdin88 deleted the feat/permission-manager branch April 27, 2026 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants